Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request introduces a "super chapters" feature enabling hierarchical organization of release notes by grouping regular chapters under higher-level category headings. The implementation adds new action inputs ( Changes
Sequence DiagramsequenceDiagram
participant Action as GitHub Action
participant Input as ActionInputs
participant CC as CustomChapters
participant HRecord as HierarchyIssueRecord
participant Output as Release Notes
Action->>Input: get_super_chapters()
Input->>Input: Parse YAML, validate entries
Input-->>Action: Return list[dict] with{title, labels}
Action->>CC: __init__(super_chapters=...)
CC->>CC: Initialize _super_chapters list
Action->>CC: populate(records)
CC->>CC: Track record labels via get_labels()
Action->>CC: to_string()
alt Has super chapters
CC->>CC: For each super chapter by label
CC->>HRecord: to_chapter_row(label_filter=super_labels)
HRecord->>HRecord: has_matching_labels(label_filter)
HRecord->>HRecord: Filter sub-hierarchy/sub-issues by labels
HRecord-->>CC: Filtered markdown row
CC->>Output: Render nested heading structure
CC->>CC: Collect unmatched records
CC->>Output: Render ## Uncategorized section
else No super chapters
CC->>Output: Render flat chapter structure
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for super chapters to the release notes generator, allowing users to group existing custom chapters under higher-level ## headings based on labels, while preserving the original flat ### rendering when the feature is not configured.
Changes:
- Introduces a new optional
super-chaptersaction input (YAML) and wires it throughActionInputs. - Extends
CustomChaptersrendering to support super-chapter grouping plus an## Uncategorizedfallback. - Adds unit tests and updates documentation/action metadata for the new input.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
action.yml |
Adds the super-chapters input and exports it as INPUT_SUPER_CHAPTERS for runtime consumption. |
release_notes_generator/action_inputs.py |
Implements ActionInputs.get_super_chapters() and logs parsed super-chapter config during validation. |
release_notes_generator/chapters/custom_chapters.py |
Adds super-chapter parsing and grouped rendering logic with an Uncategorized fallback. |
release_notes_generator/utils/constants.py |
Adds SUPER_CHAPTERS input key constant and extracts UNCATEGORIZED_CHAPTER_TITLE. |
docs/features/custom_chapters.md |
Documents configuration, rendering, behavior, and validation rules for super chapters. |
docs/configuration_reference.md |
Adds super-chapters to the inputs reference table. |
tests/unit/release_notes_generator/test_action_inputs.py |
Adds unit tests covering parsing/validation behavior for get_super_chapters(). |
tests/unit/release_notes_generator/chapters/test_custom_chapters.py |
Adds unit tests for grouped super-chapter output behavior and fallback behavior. |
tests/unit/conftest.py |
Adds a helper to construct CustomChapters instances with super-chapter behavior patched in tests. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
action.yml (1)
33-38: Consider documenting uncategorized fallback in the input description.You already document matching and multi-membership; adding one line about the automatic
Uncategorizedsection would make behavior complete directly inaction.yml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@action.yml` around lines 33 - 38, Update the description for the super-chapters input in action.yml to mention the automatic fallback: add a sentence to the existing description block (the `description` key for the super-chapter array) stating that any records not matching any super-chapter label will be placed into an automatic "Uncategorized" section; keep it brief and match the tone of the surrounding bullets.release_notes_generator/chapters/custom_chapters.py (1)
235-244: Temporary mutation ofchapter.rowsis a code smell.While the
try/finallyensures restoration, directly mutating shared state can be fragile and isn't thread-safe. Consider either passing filter criteria toChapter.to_stringor creating a lightweight wrapper that yields filtered rows without mutation.Also, the dual check
str(rid) in matching_ids or rid in matching_idssuggests type inconsistency between row IDs (possibly int) andmatching_ids(always str). Consider normalizing ID types upstream for cleaner comparisons.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/chapters/custom_chapters.py` around lines 235 - 244, The helper _render_chapter_for_ids currently mutates chapter.rows; instead avoid shared-state mutation by building a filtered_rows dict and invoking Chapter.to_string on a non-mutated object—either (A) add an optional parameter to Chapter.to_string like rows/filter_ids and pass filtered_rows, or (B) create a lightweight shallow copy of the Chapter (e.g., new_chapter = Chapter(...same metadata..., rows=filtered_rows) or add a Chapter.with_rows(rows) factory) and call new_chapter.to_string; also normalize ID comparisons by converting matching_ids to a consistent type (e.g., set[str] = {str(x) for x in matching_ids}) or casting rid once when creating filtered_rows so you can remove the dual check (str(rid) in matching_ids or rid in matching_ids).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/custom_chapters.md`:
- Line 258: Update the sentence "Within each super chapter, records are routed
to regular chapters by the normal label matching rules." to use a hyphenated
compound adjective for clarity — change "label matching rules" to
"label-matching rules" so the line reads: "Within each super chapter, records
are routed to regular chapters by the normal label-matching rules."
In `@release_notes_generator/action_inputs.py`:
- Around line 175-195: get_super_chapters currently only YAML-parses the input
and leaves semantic validation to CustomChapters._parse_super_chapters; move
that full validation into the ActionInputs boundary by having get_super_chapters
perform the same semantic checks and return only validated structures
(list[dict[str,str]]). Specifically, extend get_super_chapters to validate each
entry's type and required keys/fields (the same contract enforced in
CustomChapters._parse_super_chapters), log and skip or error on invalid entries
consistently, and ensure the return value is strictly the validated list so
CustomChapters no longer needs to re-validate; update/remove the redundant
checks in CustomChapters._parse_super_chapters accordingly.
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 367-399: Change the _parse_super_chapters signature to accept
mixed YAML types (use list[dict[str, Any]] for raw_super_chapters) and validate
the title before using it: after extracting entry["title"] in
_parse_super_chapters, ensure it's a str (if not, logger.warning and continue),
and keep using SuperChapter(title=title, labels=...) and _normalize_labels as
before; also adjust the local typing for labels_input to reflect str | list[str]
if you keep annotations.
---
Nitpick comments:
In `@action.yml`:
- Around line 33-38: Update the description for the super-chapters input in
action.yml to mention the automatic fallback: add a sentence to the existing
description block (the `description` key for the super-chapter array) stating
that any records not matching any super-chapter label will be placed into an
automatic "Uncategorized" section; keep it brief and match the tone of the
surrounding bullets.
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 235-244: The helper _render_chapter_for_ids currently mutates
chapter.rows; instead avoid shared-state mutation by building a filtered_rows
dict and invoking Chapter.to_string on a non-mutated object—either (A) add an
optional parameter to Chapter.to_string like rows/filter_ids and pass
filtered_rows, or (B) create a lightweight shallow copy of the Chapter (e.g.,
new_chapter = Chapter(...same metadata..., rows=filtered_rows) or add a
Chapter.with_rows(rows) factory) and call new_chapter.to_string; also normalize
ID comparisons by converting matching_ids to a consistent type (e.g., set[str] =
{str(x) for x in matching_ids}) or casting rid once when creating filtered_rows
so you can remove the dual check (str(rid) in matching_ids or rid in
matching_ids).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b625f1e8-a06c-426d-9639-b30f9cd8a5dc
📒 Files selected for processing (9)
action.ymldocs/configuration_reference.mddocs/features/custom_chapters.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/utils/constants.pytests/unit/conftest.pytests/unit/release_notes_generator/chapters/test_custom_chapters.pytests/unit/release_notes_generator/test_action_inputs.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@release_notes_generator/action_inputs.py`:
- Around line 208-223: The code currently accepts any non-missing "title" value
which may be non-string or blank; update the validation around entry["title"] in
the super-chapter parsing block so that you only accept a title that is a
non-empty string (e.g., check isinstance(title, str) and title.strip() != ""),
log a warning and continue if the title is invalid, and only pass the
cleaned/stripped title into result.append({"title": title, "labels":
normalized}) (keep using normalize_labels and the existing logging patterns).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f83123e6-7884-4ada-81f2-3e9f9139277a
📒 Files selected for processing (7)
docs/features/custom_chapters.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/utils/utils.pytests/unit/conftest.pytests/unit/release_notes_generator/chapters/test_custom_chapters.pytests/unit/release_notes_generator/test_action_inputs.py
✅ Files skipped from review due to trivial changes (1)
- docs/features/custom_chapters.md
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/unit/release_notes_generator/test_action_inputs.py
- tests/unit/conftest.py
- tests/unit/release_notes_generator/chapters/test_custom_chapters.py
- release_notes_generator/chapters/custom_chapters.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
release_notes_generator/action_inputs.py (1)
211-226:⚠️ Potential issue | 🟡 MinorStrip the validated title before appending.
Line 212 validates that
title.strip()is non-empty, but Line 226 appends the originaltitlevalue with potential leading/trailing whitespace. This could produce headings like" Module A "in the release notes.Proposed fix
title = entry["title"] if not isinstance(title, str) or not title.strip(): logger.warning("Skipping super-chapter with invalid title value: %r", title) continue + title = title.strip() raw_labels = entry.get("labels", entry.get("label"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/action_inputs.py` around lines 211 - 226, The validated title may contain leading/trailing whitespace—after confirming title.strip() is non-empty, replace or assign a stripped version (e.g., title = title.strip() or use title_stripped) before appending; update the code that builds the output (the result.append({"title": title, "labels": normalized}) call) to use the stripped title so headings are normalized, leaving label normalization via normalize_labels unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@release_notes_generator/action_inputs.py`:
- Around line 211-226: The validated title may contain leading/trailing
whitespace—after confirming title.strip() is non-empty, replace or assign a
stripped version (e.g., title = title.strip() or use title_stripped) before
appending; update the code that builds the output (the result.append({"title":
title, "labels": normalized}) call) to use the stripped title so headings are
normalized, leaving label normalization via normalize_labels unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4be0854c-caad-4b3c-8aab-3e5bef3e0bb2
📒 Files selected for processing (2)
release_notes_generator/action_inputs.pytests/unit/release_notes_generator/test_action_inputs.py
✅ Files skipped from review due to trivial changes (1)
- tests/unit/release_notes_generator/test_action_inputs.py
…ents (#280) * feat: enhance hierarchy issue rendering logic for open and closed parents * feat: add support for highlighting open sub-issues under closed hierarchy parents
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/unit/release_notes_generator/utils/test_gh_action.py (1)
22-36: Cover the explicitdefault=path too.These assertions lock in the implicit
default=""call, but the contract change also depends on forwarding a caller-supplied default unchanged. A smallget_action_input("explicit-default", "fallback")case would keep that branch covered.🧪 Suggested test
+def test_get_input_with_explicit_default(mocker): + mock_getenv = mocker.patch("os.getenv", side_effect=lambda name, default=None: default) + + result = get_action_input("explicit-default", "fallback") + + mock_getenv.assert_called_with("INPUT_EXPLICIT_DEFAULT", default="fallback") + assert result == "fallback"As per coding guidelines, "Test return values, exceptions, log messages, and exit codes in Python tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/release_notes_generator/utils/test_gh_action.py` around lines 22 - 36, Add a test that exercises the explicit default path for get_action_input: call get_action_input("explicit-default", "fallback") in tests/unit/release_notes_generator/utils/test_gh_action.py, mock os.getenv and assert it was called with the environment key "INPUT_EXPLICIT_DEFAULT" and default="fallback", and assert the function returns the mocked value; reference get_action_input to locate the implementation and the existing tests for pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/custom_chapters.md`:
- Around line 218-315: The docs duplicate the entire "## Super Chapters" section
back-to-back; remove the second copy so there's a single consolidated "## Super
Chapters" section (look for the repeated "## Super Chapters" heading). In that
remaining section add a short sentence to Rendering or Behavior stating the
fallback: "Records that do not match any super-chapter labels are grouped under
## Uncategorized." Also ensure the Validation paragraph still appears once and
mentions skipped entries as before.
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 421-425: The constructor is re-parsing already-validated
super-chapter entries causing duplicated validation; replace the duplicate calls
so self._super_chapters is assigned the validated, normalized output from
ActionInputs.get_super_chapters() only (do not call _parse_super_chapters on
that value), remove the redundant parsing calls around the constructor and the
block referenced (including the repeated section covering the 429-474 region),
and leave the standalone _parse_super_chapters method only for the original
raw-YAML parsing path if still needed; target symbols: self._super_chapters,
_parse_super_chapters, and ActionInputs.get_super_chapters.
- Around line 91-99: The class contains unresolved duplicate members—remove the
later duplicates and keep only the first occurrence of each: delete the second
__init__ duplicate and the later definitions of _to_string_with_super_chapters
and _parse_super_chapters; ensure the kept _to_string_with_super_chapters is the
first version that preserves the "## Uncategorized" fallback logic so unmatched
records are collected, and ensure the kept _parse_super_chapters is the first
version that does not re-validate/normalize input (leaving validation to
ActionInputs.get_super_chapters()); update imports/types only if needed after
removing the duplicates.
In `@release_notes_generator/model/record/hierarchy_issue_record.py`:
- Around line 225-254: The code that prefixes open-child icons currently
prepends the icon before the child's existing "- " list marker (see
sub_hierarchy_issue.to_chapter_row() handling and the open_icon_prefix /
sub_issue_block construction), which breaks nested Markdown and leaves stray
spaces when the icon is empty; change the logic to detect and preserve an
existing leading list marker (the initial "-" and following space) and insert
the icon immediately after that marker (e.g., "- " -> "- {icon} " only when icon
non-empty, otherwise leave as "- "), update both the sub_hierarchy_issue branch
and the sub_issue block where open_icon_prefix is used
(ActionInputs.get_open_hierarchy_sub_issue_icon()), and add regression
assertions that verify the rendered output begins with "- 🟡" when icon set and
remains "- " when icon is empty.
---
Nitpick comments:
In `@tests/unit/release_notes_generator/utils/test_gh_action.py`:
- Around line 22-36: Add a test that exercises the explicit default path for
get_action_input: call get_action_input("explicit-default", "fallback") in
tests/unit/release_notes_generator/utils/test_gh_action.py, mock os.getenv and
assert it was called with the environment key "INPUT_EXPLICIT_DEFAULT" and
default="fallback", and assert the function returns the mocked value; reference
get_action_input to locate the implementation and the existing tests for
pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4d7d817-034a-47e2-933f-b6b4ec8142c4
📒 Files selected for processing (11)
action.ymldocs/features/custom_chapters.mddocs/features/issue_hierarchy_support.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/model/record/hierarchy_issue_record.pyrelease_notes_generator/utils/constants.pyrelease_notes_generator/utils/gh_action.pytests/unit/conftest.pytests/unit/release_notes_generator/model/test_hierarchy_issue_record.pytests/unit/release_notes_generator/utils/test_gh_action.py
🚧 Files skipped from review as they are similar to previous changes (4)
- action.yml
- release_notes_generator/action_inputs.py
- release_notes_generator/utils/constants.py
- tests/unit/conftest.py
…eRecord classes for improved readability
miroslavpojer
added
the
work in progress
…d classes for clarity
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
release_notes_generator/chapters/custom_chapters.py (1)
117-148:⚠️ Potential issue | 🟠 MajorUse one label source for hierarchy routing.
Line 117 reads
record.labelsearly and uses it for regular chapter routing (line 147). Line 123 separately computesrecord.get_labels()for super-chapter tracking. ForHierarchyIssueRecordinstances constructed withissue_labels(which always happens indefault_record_factory.py), these diverge:record.labelsreturns the cached original labels whilerecord.get_labels()returns fresh aggregated labels from descendants. A record can then be claimed by a super-chapter via descendants without ever landing in any regular chapter row, causing it to silently disappear from grouped output.Restructure to use the same label source for all routing:
Suggested fix
- record_labels = getattr(record, "labels", []) - - # Track labels for super-chapter grouping at render time. - # HierarchyIssueRecords use aggregated labels (own + descendants) so the - # hierarchy parent matches super chapters via sub-issue labels. - if isinstance(record, HierarchyIssueRecord): - all_labels = record.get_labels() - if all_labels: - self._record_labels[record_id] = list(all_labels) - elif record_labels: + if isinstance(record, HierarchyIssueRecord): + record_labels = record.get_labels() + else: + record_labels = getattr(record, "labels", []) + + if record_labels: self._record_labels[record_id] = list(record_labels)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/chapters/custom_chapters.py` around lines 117 - 148, The code reads record.labels into record_labels but separately calls record.get_labels() for HierarchyIssueRecord when populating self._record_labels, causing divergence between regular chapter routing and super-chapter grouping; unify the label source by computing labels = record.get_labels() for HierarchyIssueRecord (and falling back to record.labels only for non-hierarchy records) and then use that single labels variable for: assigning self._record_labels, passing into _try_route_to_coh_chapter, the record_labels truthiness check, and matching against ch.labels in the loop that calls _add_record_to_chapter; update references to record_labels throughout this block to the unified labels variable so HierarchyIssueRecord uses aggregated labels consistently.
♻️ Duplicate comments (1)
release_notes_generator/model/record/hierarchy_issue_record.py (1)
323-327:⚠️ Potential issue | 🟡 MinorHandle the empty icon the same way for direct sub-issues.
This branch still builds
" "whenopen-hierarchy-sub-issue-iconis"", so direct sub-issues render as- ...even though the sub-hierarchy path was already fixed.Suggested fix
open_icon_prefix = "" if self.is_closed and sub_issue.is_open: # Highlight open children under a closed parent to signal incomplete work - open_icon_prefix = ActionInputs.get_open_hierarchy_sub_issue_icon() + " " + icon = ActionInputs.get_open_hierarchy_sub_issue_icon() + open_icon_prefix = f"{icon} " if icon else ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@release_notes_generator/model/record/hierarchy_issue_record.py` around lines 323 - 327, The code builds an "open_icon_prefix" of " " even when ActionInputs.get_open_hierarchy_sub_issue_icon() returns an empty string, causing direct sub-issues to render with an extra space; change the logic in the block that sets open_icon_prefix (used when checking self.is_closed and sub_issue.is_open) to only prepend the icon plus a trailing space when the retrieved icon is non-empty (i.e., check the icon string truthiness before concatenating), so sub_issue_block = "- " + open_icon_prefix + sub_issue.to_chapter_row() will not include a stray space when the icon is "".
🧹 Nitpick comments (1)
tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py (1)
730-731: Avoid_labelswrites in the hierarchy split tests.Writing
HierarchyIssueRecord._labelsdirectly bypasses the public label-loading path and the cache behavior these cases are trying to validate, so it can hide real regressions. Prefer constructing the record withissue_labels=or mockingissue.get_labels()instead. As per coding guidelines, "Must not access private members (names starting with_) of the class under test directly in tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py` around lines 730 - 731, The test directly assigns to HierarchyIssueRecord._labels which bypasses the public label-loading path and cached behavior; instead construct the record with the public issue_labels parameter (e.g. HierarchyIssueRecord(..., issue_labels=["security"])) or mock the underlying issue.get_labels() on the object returned by make_minimal_issue so the label-loading path exercised by HierarchyIssueRecord is used; update the test to remove the _labels write and supply labels via issue_labels or a mocked get_labels to preserve cache/behavior verification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@release_notes_generator/chapters/custom_chapters.py`:
- Around line 117-148: The code reads record.labels into record_labels but
separately calls record.get_labels() for HierarchyIssueRecord when populating
self._record_labels, causing divergence between regular chapter routing and
super-chapter grouping; unify the label source by computing labels =
record.get_labels() for HierarchyIssueRecord (and falling back to record.labels
only for non-hierarchy records) and then use that single labels variable for:
assigning self._record_labels, passing into _try_route_to_coh_chapter, the
record_labels truthiness check, and matching against ch.labels in the loop that
calls _add_record_to_chapter; update references to record_labels throughout this
block to the unified labels variable so HierarchyIssueRecord uses aggregated
labels consistently.
---
Duplicate comments:
In `@release_notes_generator/model/record/hierarchy_issue_record.py`:
- Around line 323-327: The code builds an "open_icon_prefix" of " " even when
ActionInputs.get_open_hierarchy_sub_issue_icon() returns an empty string,
causing direct sub-issues to render with an extra space; change the logic in the
block that sets open_icon_prefix (used when checking self.is_closed and
sub_issue.is_open) to only prepend the icon plus a trailing space when the
retrieved icon is non-empty (i.e., check the icon string truthiness before
concatenating), so sub_issue_block = "- " + open_icon_prefix +
sub_issue.to_chapter_row() will not include a stray space when the icon is "".
---
Nitpick comments:
In `@tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py`:
- Around line 730-731: The test directly assigns to HierarchyIssueRecord._labels
which bypasses the public label-loading path and cached behavior; instead
construct the record with the public issue_labels parameter (e.g.
HierarchyIssueRecord(..., issue_labels=["security"])) or mock the underlying
issue.get_labels() on the object returned by make_minimal_issue so the
label-loading path exercised by HierarchyIssueRecord is used; update the test to
remove the _labels write and supply labels via issue_labels or a mocked
get_labels to preserve cache/behavior verification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a7cf56c-bad0-4a7f-a2fe-e4c34e0860d7
📒 Files selected for processing (10)
.github/copilot-instructions.mddocs/features/custom_chapters.mddocs/features/issue_hierarchy_support.mdrelease_notes_generator/action_inputs.pyrelease_notes_generator/chapters/custom_chapters.pyrelease_notes_generator/data/utils/bulk_sub_issue_collector.pyrelease_notes_generator/model/record/hierarchy_issue_record.pytests/unit/release_notes_generator/chapters/test_custom_chapters.pytests/unit/release_notes_generator/model/test_hierarchy_issue_record.pytests/unit/release_notes_generator/test_action_inputs.py
✅ Files skipped from review due to trivial changes (2)
- docs/features/issue_hierarchy_support.md
- .github/copilot-instructions.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/release_notes_generator/test_action_inputs.py
tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py
Outdated
Show resolved
Hide resolved
tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py
Outdated
Show resolved
Hide resolved
tests/unit/release_notes_generator/model/test_hierarchy_issue_record.py
Outdated
Show resolved
Hide resolved
tests/unit/release_notes_generator/chapters/test_custom_chapters.py
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds
super-chaptersas a new optional input that lets users group existing custom chapters under top-level##headings in the release notes output.Previously, all custom chapters rendered as flat
###headings. With super chapters, users can declare label-based groupings (e.g. "Module A", "Module B") that wrap the matching records under a##heading, with the regular chapter titles nested as###inside. Records not matched by any super chapter fall through to an## Uncategorizedsection.Key changes:
ActionInputs.get_super_chapters()parses the newsuper-chaptersYAML input (mirrorsget_chapters()pattern)CustomChapters._to_string_with_super_chapters()refactored into focused helpers and extended with an uncategorized fallback_record_labelsnow tracks all records (including label-less COH-routed ones) so they appear in the fallbackUNCATEGORIZED_CHAPTER_TITLEconstant extracted to keep the contract-visible string stableRelease Notes
super-chaptersinput: group release note chapters under top-level##headings by label## Uncategorizedfallback sectionRelated
Closes #99
Summary by CodeRabbit
New Features
Documentation